Fix double free in DecompressedData init failure path (DCTAnimationCacheImpl)#2221
Open
ankuper wants to merge 1 commit into
Open
Fix double free in DecompressedData init failure path (DCTAnimationCacheImpl)#2221ankuper wants to merge 1 commit into
ankuper wants to merge 1 commit into
Conversation
…failed init compression_stream_init() failure was manually deallocating self.stream before returning nil. Since all stored properties are already assigned by that point, Swift still calls deinit on the failed-init instance, which unconditionally destroys+deallocates self.stream again — a double free that libsystem_malloc catches as "pointer being freed was not allocated" (SIGABRT, DCTMultiAnimationRenderer-FirstFrame queue, build 33197). Let deinit be the sole owner of that cleanup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
DecompressedData.init?(compressedData:dataRange:)inDCTAnimationCacheImpl.swiftallocatesself.streamand, ifcompression_stream_initfails, manually deallocates it before returningnil:By the point
return nilexecutes, every stored property (compressedData,dataRange, andstream) has already been assigned. Under Swift's documented behavior for failable initializers, once all stored properties are set,deinitstill runs on the instance even thoughinit?returnednil.deinitunconditionally does:So
self.streamgets deallocated twice: once explicitly in theinit?failure branch, and again fromdeinit.compression_stream_initzeroes the stream's non-statefields per its own documentation, but that doesn't change the fact that the outerUnsafeMutablePointer<compression_stream>allocation itself is freed twice — a double free.Crash
Observed on-device on iOS 26.5 (23F77), SIGABRT,
libsystem_malloc.dylibreporting:Faulting thread was on queue
DCTMultiAnimationRenderer-FirstFrame, unwinding throughswift_release_dealloc→DecompressedData.__deallocating_deinit→DecompressedData.deinit, withDecompressedData.init(compressedData:dataRange:)further up the same call chain — consistent withdeinitrunning a second time on the stream pointer thatinit?'s failure branch had already freed.Fix
Remove the manual
self.stream.deallocate()from theinit?failure branch.deinitalready destroys and deallocatesself.streamunconditionally, and per Swift's ARC semantics it will run exactly once for this instance regardless of whetherinit?returnednil(once stored properties are set) or completed successfully. This makesdeinitthe single owner of that cleanup, eliminating the double free.Testing
atosagainst the release dSYM) that the pre-fix code path matches this exact double-free mechanism.master(not something we introduced downstream).